feat: add lapack/base/dlarf#12331
Conversation
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown_pkg_readmes
status: passed
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- task: lint_package_json
status: passed
- task: lint_repl_help
status: passed
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: passed
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: passed
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: passed
- task: lint_license_headers
status: passed
---
Coverage Report
The above coverage report was generated for the changes in this PR. |
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown_pkg_readmes
status: passed
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown_pkg_readmes
status: passed
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
| * @param {Float64Array} work - workspace array | ||
| * @throws {TypeError} first argument must be a valid order | ||
| * @throws {TypeError} second argument must be a valid side | ||
| * @throws {RangeError} ninth argument must be greater than or equal to max(1,N) |
There was a problem hiding this comment.
* @throws {RangeError} ninth argument must be greater than or equal to max(1,N)
That only applies to row-major. Column-major has no LDC check in code, and LAPACK expects LDC >= max(1, M) for column-major.
There was a problem hiding this comment.
@iampratik13 Mind verifying this commit if this looks good? Thanks!
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown_pkg_readmes
status: passed
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: passed
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
| ``` | ||
|
|
||
| The function has the following additional parameters: | ||
| The function has the following parameters: |
There was a problem hiding this comment.
They are not "additional"
| // Scan for the last non-zero row in C(:,0:lastv-1): | ||
| lastc = iladlr( M, lastv, C, strideC1, strideC2, offsetC ) + 1; // adjust by +1 to account for the difference between zero-based and one-based indexing | ||
| } | ||
| // Note that lastc === 0 renders the BLAS operations null; no special case is needed at this level... |
There was a problem hiding this comment.
| // Note that lastc === 0 renders the BLAS operations null; no special case is needed at this level... | |
| // Return `C` unchanged if all elements in `C` are zero... | |
| if ( lastc === 0 ) { | |
| return C; | |
| } |
I think it's better to have an early return instead.
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown_pkg_readmes
status: na
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
| * @throws {RangeError} if `order` is `'row-major'`, ninth argument must be greater than or equal to max(1,N) | ||
| * @throws {RangeError} if `order` is `'column-major'`, ninth argument must be greater than or equal to max(1,M) |
There was a problem hiding this comment.
| * @throws {RangeError} if `order` is `'row-major'`, ninth argument must be greater than or equal to max(1,N) | |
| * @throws {RangeError} if `order` is `'column-major'`, ninth argument must be greater than or equal to max(1,M) | |
| * @throws {RangeError} ninth argument must be a valid stride |
There was a problem hiding this comment.
It is okay to be a bit more general in throws descriptions.
| if ( isRowMajor( order ) && LDC < max( 1, N ) ) { | ||
| throw new RangeError( format( 'invalid argument. Ninth argument must be greater than or equal to max(1,%d). Value: `%d`.', N, LDC ) ); | ||
| } | ||
| if ( isColumnMajor( order ) && LDC < max( 1, M ) ) { | ||
| throw new RangeError( format( 'invalid argument. Ninth argument must be greater than or equal to max(1,%d). Value: `%d`.', M, LDC ) ); | ||
| } | ||
| if ( isColumnMajor( order ) ) { | ||
| sc1 = 1; | ||
| sc2 = LDC; | ||
| } else { // order === 'row-major' | ||
| sc1 = LDC; | ||
| sc2 = 1; | ||
| } |
There was a problem hiding this comment.
This can be consolidated. Avoid unnecessary and duplicate branching.
| if ( isRowMajor( order ) && LDC < max( 1, N ) ) { | |
| throw new RangeError( format( 'invalid argument. Ninth argument must be greater than or equal to max(1,%d). Value: `%d`.', N, LDC ) ); | |
| } | |
| if ( isColumnMajor( order ) && LDC < max( 1, M ) ) { | |
| throw new RangeError( format( 'invalid argument. Ninth argument must be greater than or equal to max(1,%d). Value: `%d`.', M, LDC ) ); | |
| } | |
| if ( isColumnMajor( order ) ) { | |
| sc1 = 1; | |
| sc2 = LDC; | |
| } else { // order === 'row-major' | |
| sc1 = LDC; | |
| sc2 = 1; | |
| } | |
| if ( isColumnMajor( order ) ) { | |
| if ( LDC < max( 1, M ) ) { | |
| throw new RangeError( format( 'invalid argument. Ninth argument must be greater than or equal to max(1,%d). Value: `%d`.', M, LDC ) ); | |
| } | |
| sc1 = 1; | |
| sc2 = LDC; | |
| } else { // order === 'row-major' | |
| if ( LDC < max( 1, N ) ) { | |
| throw new RangeError( format( 'invalid argument. Ninth argument must be greater than or equal to max(1,%d). Value: `%d`.', N, LDC ) ); | |
| } | |
| sc1 = LDC; | |
| sc2 = 1; | |
| } |
It is likely we need to touch up some other packages to reduce branching and duplicate calls. Feel free to open a PR fixing this.
| // w(0:lastc-1) := C(0:lastv-1, 0:lastc-1)^T * v(0:lastv-1) | ||
| dgemv( 'transpose', lastv, lastc, 1.0, C, strideC1, strideC2, offsetC, V, strideV, offsetV, 0.0, work, strideWork, offsetWork ); | ||
|
|
||
| // C(0:lastv-1, 0:lastc-1) := C(...) - tau * v(0:lastv-1) * w(0:lastc-1)^T | ||
| dger( lastv, lastc, -tau, V, strideV, offsetV, work, strideWork, offsetWork, C, strideC1, strideC2, offsetC ); |
There was a problem hiding this comment.
| // w(0:lastc-1) := C(0:lastv-1, 0:lastc-1)^T * v(0:lastv-1) | |
| dgemv( 'transpose', lastv, lastc, 1.0, C, strideC1, strideC2, offsetC, V, strideV, offsetV, 0.0, work, strideWork, offsetWork ); | |
| // C(0:lastv-1, 0:lastc-1) := C(...) - tau * v(0:lastv-1) * w(0:lastc-1)^T | |
| dger( lastv, lastc, -tau, V, strideV, offsetV, work, strideWork, offsetWork, C, strideC1, strideC2, offsetC ); | |
| // work[0:lastc-1] := C[0:lastv-1, 0:lastc-1]^T * V[0:lastv-1] | |
| dgemv( 'transpose', lastv, lastc, 1.0, C, strideC1, strideC2, offsetC, V, strideV, offsetV, 0.0, work, strideWork, offsetWork ); | |
| // C[0:lastv-1, 0:lastc-1] := C[...] - tau * V[0:lastv-1] * work[0:lastc-1]^T | |
| dger( lastv, lastc, -tau, V, strideV, offsetV, work, strideWork, offsetWork, C, strideC1, strideC2, offsetC ); |
Keeping them consistent with our variable names, and how we write similar comments in other reference implementations.
| // w(0:lastc-1) := C(0:lastc-1, 0:lastv-1) * v(0:lastv-1) | ||
| dgemv( 'no-transpose', lastc, lastv, 1.0, C, strideC1, strideC2, offsetC, V, strideV, offsetV, 0.0, work, strideWork, offsetWork ); | ||
|
|
||
| // C(0:lastc-1, 0:lastv-1) := C(...) - tau * w(0:lastc-1) * v(0:lastv-1)^T | ||
| dger( lastc, lastv, -tau, work, strideWork, offsetWork, V, strideV, offsetV, C, strideC1, strideC2, offsetC ); |
There was a problem hiding this comment.
| // w(0:lastc-1) := C(0:lastc-1, 0:lastv-1) * v(0:lastv-1) | |
| dgemv( 'no-transpose', lastc, lastv, 1.0, C, strideC1, strideC2, offsetC, V, strideV, offsetV, 0.0, work, strideWork, offsetWork ); | |
| // C(0:lastc-1, 0:lastv-1) := C(...) - tau * w(0:lastc-1) * v(0:lastv-1)^T | |
| dger( lastc, lastv, -tau, work, strideWork, offsetWork, V, strideV, offsetV, C, strideC1, strideC2, offsetC ); | |
| // work[0:lastc-1] := C[0:lastc-1, 0:lastv-1] * V[0:lastv-1] | |
| dgemv( 'no-transpose', lastc, lastv, 1.0, C, strideC1, strideC2, offsetC, V, strideV, offsetV, 0.0, work, strideWork, offsetWork ); | |
| // C[0:lastc-1, 0:lastv-1] := C[...] - tau * work[0:lastc-1] * V[0:lastv-1]^T | |
| dger( lastc, lastv, -tau, work, strideWork, offsetWork, V, strideV, offsetV, C, strideC1, strideC2, offsetC ); |
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown_pkg_readmes
status: na
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes. report:
Resolves none.
Description
This pull request:
lapack/base/dlarfRelated Issues
This pull request has the following related issues:
Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
{{TODO: add disclosure if applicable}}
@stdlib-js/reviewers